Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

On-behalf-of token and service account authorization tokens doc #5123

Merged
merged 69 commits into from
Feb 1, 2024

Conversation

stephen-crawford
Copy link
Contributor

@stephen-crawford stephen-crawford commented Oct 2, 2023

Description

This PR introduces a new document inside the security plugin's access_control directory. The document added details the basic information for On-behalf-of and Service Account tokens. I reviewed the documentation provided for accuracy from a security standpoint and as a doc bar raiser. Hopefully there should not be many further changes required.

I was not sure how to change the linking correctly but did my best.

Issues Resolved

opensearch-project/security#3290
Fixes #4388

Checklist

  • By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license and subject to the Developers Certificate of Origin.
    For more information on following Developer Certificate of Origin and signing off your commits, please check here.

@hdhalter
Copy link
Contributor

hdhalter commented Oct 2, 2023

@Naarcha-AWS - Can you please take this over now that Chris is gone? Thanks.

@hdhalter
Copy link
Contributor

hdhalter commented Nov 3, 2023

@scrawfor99 - did you close this in favor of #4411?

@stephen-crawford
Copy link
Contributor Author

stephen-crawford commented Nov 7, 2023

@hdhalter, I definitely did not mean to close this... Let me see what I did by accident

Update: should be fixed. Sorry about that looks like my rebase wiped it and I did not notice

Signed-off-by: Stephen Crawford <[email protected]>
@hdhalter hdhalter added the 3 - Tech review PR: Tech review in progress label Nov 7, 2023
@stephen-crawford
Copy link
Contributor Author

@hdhalter I am not sure what that issue with the style check means :/

Signed-off-by: Stephen Crawford <[email protected]>
@stephen-crawford
Copy link
Contributor Author

The remaining vale warnings I do not plan to address unless required for the documentation to be merged. It is stuff around the headers having numbers etc.

Copy link
Collaborator

@kolchfa-aws kolchfa-aws left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you, @scrawfor99! Left some comments.

_security/access-control/authentication-tokens.md Outdated Show resolved Hide resolved
_security/access-control/authentication-tokens.md Outdated Show resolved Hide resolved
_security/access-control/authentication-tokens.md Outdated Show resolved Hide resolved
_security/access-control/authentication-tokens.md Outdated Show resolved Hide resolved
_security/access-control/authentication-tokens.md Outdated Show resolved Hide resolved
_security/access-control/authentication-tokens.md Outdated Show resolved Hide resolved
_security/access-control/authentication-tokens.md Outdated Show resolved Hide resolved
_security/access-control/authentication-tokens.md Outdated Show resolved Hide resolved
_security/access-control/authentication-tokens.md Outdated Show resolved Hide resolved
_security/access-control/authentication-tokens.md Outdated Show resolved Hide resolved
@kolchfa-aws
Copy link
Collaborator

Also, could you add blank lines after each heading? Thanks!

stephen-crawford and others added 3 commits November 14, 2023 10:24
Co-authored-by: kolchfa-aws <[email protected]>
Signed-off-by: Stephen Crawford <[email protected]>
Co-authored-by: kolchfa-aws <[email protected]>
Signed-off-by: Stephen Crawford <[email protected]>
Co-authored-by: kolchfa-aws <[email protected]>
Signed-off-by: Stephen Crawford <[email protected]>
@stephen-crawford
Copy link
Contributor Author

I don't see how to ignore the review dog comments that remain. It wants to mess with all the headers etc. but I think they are good

@hdhalter hdhalter added 6 - Done but waiting to merge PR: The work is done and ready to merge and removed 3 - Tech review PR: Tech review in progress labels Nov 20, 2023
Copy link
Collaborator

@kolchfa-aws kolchfa-aws left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, @scrawfor99! A couple of changes before we can mark this PR done.

_security/access-control/authentication-tokens.md Outdated Show resolved Hide resolved
_security/access-control/authentication-tokens.md Outdated Show resolved Hide resolved
@kolchfa-aws kolchfa-aws added 4 - Doc review PR: Doc review in progress and removed 6 - Done but waiting to merge PR: The work is done and ready to merge labels Nov 24, 2023
@stephen-crawford
Copy link
Contributor Author

@kolchfa-aws should be all set!

@kolchfa-aws
Copy link
Collaborator

Thank you, @scrawfor99!

@kolchfa-aws kolchfa-aws added the 6 - Done but waiting to merge PR: The work is done and ready to merge label Dec 1, 2023
@hdhalter hdhalter removed the 4 - Doc review PR: Doc review in progress label Dec 4, 2023
@kolchfa-aws kolchfa-aws added the release-notes PR: Include this PR in the automated release notes label Dec 12, 2023
@peternied
Copy link
Member

@kolchfa-aws Can this be merged? What is preventing this, can we get 2.12 items merged in a branch so we know they are commited?

@kolchfa-aws
Copy link
Collaborator

@peternied We usually merge right before the release. This is done to prevent merge conflicts when someone is working on the same file for the current/previous versions.

@peternied
Copy link
Member

@kolchfa-aws Thanks for the fast response. Your scenario is what branches are built for on Github, they allow collaboration and then a PR is made on 'release' day that pulls in all the PRs on that branch.

@hdhalter hdhalter merged commit c40424f into opensearch-project:main Feb 1, 2024
3 checks passed
@hdhalter hdhalter added 3 - Done Issue is done/complete and removed 6 - Done but waiting to merge PR: The work is done and ready to merge labels Feb 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3 - Done Issue is done/complete release-notes PR: Include this PR in the automated release notes v2.12.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[DOC] Add documentation for OnBehalfOf Authentication
6 participants